Skip to content

gh-156664: Fix interaction between free variables and comprehensions - #156691

Open
JelleZijlstra wants to merge 5 commits into
python:mainfrom
JelleZijlstra:codex/gh-156664-comprehension-closure
Open

gh-156664: Fix interaction between free variables and comprehensions#156691
JelleZijlstra wants to merge 5 commits into
python:mainfrom
JelleZijlstra:codex/gh-156664-comprehension-closure

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented Aug 31, 2026

Copy link
Copy Markdown
Member

@carljm carljm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Codex found a couple regressions that this causes relative to main.

Comment thread Python/symtable.c
Comment thread Python/compile.c

@carljm carljm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Nice that this is actually a net LOC reduction, barring tests.

Codex found one remaining bug and a possible performance concern.

Comment thread Objects/frameobject.c
} else {
if (!(_PyLocals_GetKind(co->co_localspluskinds, i) & CO_FAST_HIDDEN)) {
return i;
if (framelocalsproxy_hasval(frame->f_frame, co, i)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comprehension's cell can be active but still empty before the first target assignment. In that state, this hasval() check skips it and directs a proxy write into the enclosing free-variable slot:

import sys

def values():
    sys._getframe(1).f_locals["x"] = 42
    yield 1

def outer():
    x = 7
    def inner():
        [lambda: x for x in (x, values())[1]]
    inner()
    return x

print(outer())

Main and 9c49003 print 7, while f2c0db42 prints 42. The write during iterator advancement escapes the comprehension and changes the enclosing variable.

Comment thread Objects/frameobject.c
if (framelocalsproxy_hasval(frame->f_frame, co, i)) {
size++;
}
PyObject *snapshot = framelocalsproxy_snapshot(frame);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how much performance of framelocalsproxy matters in practice, but now even frames without comprehensions or duplicate names construct and destroy a full dictionary for len(proxy). The same snapshot cost also applies to key enumeration and the other views.

In matching --with-pydebug / -Og builds, Codex measured len(proxy) with 32 ordinary locals increasing from about 0.16 µs on main to 1.10 µs here; with 128 locals, it went from 0.52 µs to 4.39 µs. Key enumeration was roughly three times slower. These are microbenchmarks taking the minimum of three runs of 20,000 operations, on a frame with no comprehensions.

If this matters, we could maintain a fast path when duplicate bindings are impossible, especially for length, which previously required no container allocation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants